Main#39
Conversation
…llar scanner, types and admin UI skeleton
…gregator Issue #26: Automated ACTA Ecosystem Activity Aggregator (initial implementation)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@copilot resolve the merge conflicts in this pull request |
📝 WalkthroughWalkthroughAdds Stellar activity monitoring storage and types, a Horizon helper for fetching and classifying operations, a Supabase Edge Function that records detected events, and an admin page that lists recent activity events. ChangesStellar activity monitoring
Sequence Diagram(s)sequenceDiagram
participant activity_scanner
participant monitored_accounts
participant fetchOperationsSince
participant "Stellar Horizon" as StellarHorizon
participant activity_events
activity_scanner->>monitored_accounts: select active accounts
loop each active account
activity_scanner->>fetchOperationsSince: request operations since cursor
fetchOperationsSince->>StellarHorizon: GET /accounts/{account}/operations
StellarHorizon-->>fetchOperationsSince: _embedded.records
fetchOperationsSince-->>activity_scanner: operations + nextCursor
activity_scanner->>activity_events: check tx_hash and insert rows
activity_scanner->>monitored_accounts: update last_cursor
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (2)
supabase/migrations/0003_monitored_accounts.sql (1)
16-17: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDrop the redundant
stellar_addressindex.
stellar_addressis alreadyUNIQUE, so Postgres creates an index for it automatically. Keeping this second one only adds write/storage overhead.Possible fix
create table if not exists public.monitored_accounts ( id uuid primary key default gen_random_uuid(), stellar_address text unique not null, @@ -create index if not exists monitored_accounts_stellar_address_idx - on public.monitored_accounts (stellar_address);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supabase/migrations/0003_monitored_accounts.sql` around lines 16 - 17, The monitored_accounts migration is creating a redundant index on stellar_address even though the column is already UNIQUE and indexed by Postgres. Remove the extra index creation from the migration in the monitored_accounts SQL so only the unique constraint-backed index remains, and keep the rest of the schema changes unchanged.supabase/migrations/0004_activity_events.sql (1)
19-20: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAdd a
detected_atindex for the admin feed.This page orders by
detected_at DESCacross the whole table, so the existing(account_id, detected_at)index won't help much. Add a dedicateddetected_at DESCindex to keep the latest-50 query from degrading as rows grow.Possible fix
create index if not exists activity_events_account_id_detected_at_idx on public.activity_events (account_id, detected_at desc); + +create index if not exists activity_events_detected_at_idx + on public.activity_events (detected_at desc);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supabase/migrations/0004_activity_events.sql` around lines 19 - 20, Add a dedicated index on detected_at for the admin feed query, since the existing activity_events_account_id_detected_at_idx only helps account-scoped lookups. Update the migration that defines the activity_events indexes to include a new index on public.activity_events using detected_at DESC, keeping the existing composite index intact and naming it clearly so it can be found alongside the current migration changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/`@types/activity.ts:
- Around line 22-34: The ActivityEvent type still uses raw_data: any, which
weakens the JSON contract and breaks linting. Update the ActivityEvent interface
in the activity types definition to use unknown or a JSON-safe payload type
instead of any, and make sure any code that reads raw_data narrows or validates
it before use.
In `@src/app/admin/`(protected)/activity/page.tsx:
- Line 11: The ActivityFeed call is bypassing the ActivityEvent type boundary by
casting events to any. Update the page component to pass a properly typed row
array into ActivityFeed by using the actual result type from the events source
and removing the unsafe cast; use the ActivityFeed and events symbols to locate
the prop wiring and preserve type safety end to end.
- Around line 7-11: The admin activity page is swallowing Supabase query
failures by falling back to an empty list in the activity feed. Update the
`page.tsx` flow around the `supabase.from('activity_events')` query and
`ActivityFeed` rendering to check for `error` and throw or otherwise surface it
instead of defaulting to `[]`, so `events` only renders when the query succeeds
and operators can distinguish an empty result from a load failure.
In `@src/lib/stellar/index.ts`:
- Line 1: Remove the unnecessary node-fetch import from the stellar module and
use the native global fetch instead. Update src/lib/stellar/index.ts by deleting
the import statement and make sure any references in the module rely on the
built-in fetch API so the code stays compatible with Next.js without requiring
an extra dependency.
- Around line 33-37: The payment classification in the logic around the t check
currently handles only payment and payment_strict_receive, so
payment_strict_send falls through to the default low-significance path. Update
the condition in the payment handling branch to include payment_strict_send, and
keep using the same amount-based significance calculation and summary formatting
in the existing payment return path.
- Around line 22-23: The Horizon request in the stellar fetch path has no
timeout, so a stalled call can block scanning. Update the fetch logic in the
stellar index flow to use an AbortController (or equivalent timeout-aware
request wrapper) with a reasonable deadline, and ensure the timeout is cleared
after completion. Keep the existing error handling around the fetch result, and
make the timeout failure surface as a clear request error from the same Horizon
request code path.
- Line 9: The `HorizonOperation` shape currently uses an explicit `any` index
signature and the map callback also relies on an unnecessary explicit `any`,
which should be removed to satisfy the lint rule. Update the `HorizonOperation`
type in `index.ts` to use `unknown` for the index signature, and remove the
explicit type annotation from the `r` parameter in the mapping logic so it can
be inferred from the array source; keep the existing `as HorizonOperation` cast
where the value is narrowed for use.
In `@supabase/functions/activity_scanner/index.ts`:
- Around line 20-27: The dedupe in activity_scanner/index.ts is too broad and
non-atomic: it currently uses transaction_hash in the existing lookup, which can
suppress distinct operations from the same transaction and can race between
concurrent runs. Update the scan/insert flow to use a Horizon operation-scoped
identifier or paging token in the activity_events record, and enforce uniqueness
on that key instead of tx_hash. Then switch the create path to an atomic
upsert/insert-with-conflict-handling around the same identifier so duplicate
scanner runs cannot insert the same operation twice.
- Around line 29-46: The activity scanner currently ignores failures from the
activity_events insert and monitored_accounts cursor update, so processed can be
incremented and last_cursor advanced even when persistence fails. In the
activity_scanner flow around the supabase.from('activity_events') insert and the
subsequent cursor update, check the returned error/result before incrementing
processed or setting last_cursor, and only advance state after a successful
write; otherwise log/handle the failure and leave the cursor unchanged so the
same events can be retried.
- Around line 19-21: Remove the redundant `as any` casts in
`activity_scanner/index.ts` by using the existing `HorizonOperation` type from
`fetchOperationsSince` directly; update the `classifyOperation` call and
`transaction_hash` access to rely on the typed operation object. Also review
`classifyOperation` in `stellar/index.ts` so dynamic fields like `account` are
accessed consistently through the interface’s index signature rather than
unnecessary casts, keeping the typing aligned across both functions.
- Around line 10-11: The monitored_accounts fetch in activity_scanner currently
treats any null data as an empty result, which hides real query failures. Update
the supabase query handling in the activity scanning flow to destructure the
error alongside data, and in the same block throw or propagate that error before
checking accounts. Keep the existing processed fallback only for the true
no-data case, and make the change in the logic around the monitored_accounts
query response.
In `@supabase/migrations/0004_activity_events.sql`:
- Around line 5-17: The activity_events table currently leaves event_type and
significance as unrestricted text, so add database-level constraints in the
migration to match the TypeScript allowed values. Update the
public.activity_events definition to constrain both columns, using CHECK
constraints or enum-backed types, and keep the change localized around the
activity_events table creation so the schema contract is enforced at the
database layer.
---
Nitpick comments:
In `@supabase/migrations/0003_monitored_accounts.sql`:
- Around line 16-17: The monitored_accounts migration is creating a redundant
index on stellar_address even though the column is already UNIQUE and indexed by
Postgres. Remove the extra index creation from the migration in the
monitored_accounts SQL so only the unique constraint-backed index remains, and
keep the rest of the schema changes unchanged.
In `@supabase/migrations/0004_activity_events.sql`:
- Around line 19-20: Add a dedicated index on detected_at for the admin feed
query, since the existing activity_events_account_id_detected_at_idx only helps
account-scoped lookups. Update the migration that defines the activity_events
indexes to include a new index on public.activity_events using detected_at DESC,
keeping the existing composite index intact and naming it clearly so it can be
found alongside the current migration changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: efdf552a-735c-4efa-b5b5-b7fc4c547764
📒 Files selected for processing (8)
.env.examplesrc/@types/activity.tssrc/app/admin/(protected)/activity/page.tsxsrc/components/modules/admin/activity/ActivityFeed.tsxsrc/lib/stellar/index.tssupabase/functions/activity_scanner/index.tssupabase/migrations/0003_monitored_accounts.sqlsupabase/migrations/0004_activity_events.sql
| export interface ActivityEvent { | ||
| id: string; | ||
| account_id: string; | ||
| source_account: string; | ||
| event_type: EventType; | ||
| significance: Significance; | ||
| raw_data: any; | ||
| summary?: string; | ||
| processed: boolean; | ||
| draft_article_id?: string | null; | ||
| detected_at: string; | ||
| tx_hash?: string | null; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace any with a JSON-safe type.
raw_data is persisted JSON, so any weakens the new contract and already fails lint. unknown (or the concrete payload shape) is safer here.
Possible fix
- raw_data: any;
+ raw_data: unknown;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export interface ActivityEvent { | |
| id: string; | |
| account_id: string; | |
| source_account: string; | |
| event_type: EventType; | |
| significance: Significance; | |
| raw_data: any; | |
| summary?: string; | |
| processed: boolean; | |
| draft_article_id?: string | null; | |
| detected_at: string; | |
| tx_hash?: string | null; | |
| } | |
| export interface ActivityEvent { | |
| id: string; | |
| account_id: string; | |
| source_account: string; | |
| event_type: EventType; | |
| significance: Significance; | |
| raw_data: unknown; | |
| summary?: string; | |
| processed: boolean; | |
| draft_article_id?: string | null; | |
| detected_at: string; | |
| tx_hash?: string | null; | |
| } |
🧰 Tools
🪛 ESLint
[error] 28-28: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🪛 GitHub Actions: CI / 0_lint-and-build.txt
[error] 28-28: ESLint (@typescript-eslint/no-explicit-any): Unexpected any. Specify a different type
🪛 GitHub Actions: CI / lint-and-build
[error] 28-28: ESLint (@typescript-eslint/no-explicit-any): Unexpected any. Specify a different type.
🪛 GitHub Check: lint-and-build
[failure] 28-28:
Unexpected any. Specify a different type
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/`@types/activity.ts around lines 22 - 34, The ActivityEvent type still
uses raw_data: any, which weakens the JSON contract and breaks linting. Update
the ActivityEvent interface in the activity types definition to use unknown or a
JSON-safe payload type instead of any, and make sure any code that reads
raw_data narrows or validates it before use.
Sources: Linters/SAST tools, Pipeline failures
| const { data: events } = await supabase.from('activity_events').select('*').order('detected_at', { ascending: false }).limit(50); | ||
| return ( | ||
| <div> | ||
| <h1>Admin — Activity</h1> | ||
| <ActivityFeed events={(events || []) as any} /> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Don't hide query failures behind an empty feed.
If the Supabase query errors, this silently renders no activity and looks like a real empty state. Throw or surface error so operators can distinguish "no data" from "failed to load data".
Possible fix
- const { data: events } = await supabase.from('activity_events').select('*').order('detected_at', { ascending: false }).limit(50);
+ const { data: events, error } = await supabase.from('activity_events').select('*').order('detected_at', { ascending: false }).limit(50);
+ if (error) throw error;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { data: events } = await supabase.from('activity_events').select('*').order('detected_at', { ascending: false }).limit(50); | |
| return ( | |
| <div> | |
| <h1>Admin — Activity</h1> | |
| <ActivityFeed events={(events || []) as any} /> | |
| const { data: events, error } = await supabase.from('activity_events').select('*').order('detected_at', { ascending: false }).limit(50); | |
| if (error) throw error; | |
| return ( | |
| <div> | |
| <h1>Admin — Activity</h1> | |
| <ActivityFeed events={(events || []) as any} /> |
🧰 Tools
🪛 ESLint
[error] 11-11: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🪛 GitHub Check: lint-and-build
[failure] 11-11:
Unexpected any. Specify a different type
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/admin/`(protected)/activity/page.tsx around lines 7 - 11, The admin
activity page is swallowing Supabase query failures by falling back to an empty
list in the activity feed. Update the `page.tsx` flow around the
`supabase.from('activity_events')` query and `ActivityFeed` rendering to check
for `error` and throw or otherwise surface it instead of defaulting to `[]`, so
`events` only renders when the query succeeds and operators can distinguish an
empty result from a load failure.
| return ( | ||
| <div> | ||
| <h1>Admin — Activity</h1> | ||
| <ActivityFeed events={(events || []) as any} /> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the as any cast.
This is the lint failure on this file, and it defeats the ActivityEvent boundary. Pass a typed row array instead of erasing the result type here.
Possible fix
- <ActivityFeed events={(events || []) as any} />
+ <ActivityFeed events={events ?? []} />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <ActivityFeed events={(events || []) as any} /> | |
| <ActivityFeed events={events ?? []} /> |
🧰 Tools
🪛 ESLint
[error] 11-11: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🪛 GitHub Check: lint-and-build
[failure] 11-11:
Unexpected any. Specify a different type
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/admin/`(protected)/activity/page.tsx at line 11, The ActivityFeed
call is bypassing the ActivityEvent type boundary by casting events to any.
Update the page component to pass a properly typed row array into ActivityFeed
by using the actual result type from the events source and removing the unsafe
cast; use the ActivityFeed and events symbols to locate the prop wiring and
preserve type safety end to end.
Sources: Linters/SAST tools, Pipeline failures
| @@ -0,0 +1,43 @@ | |||
| import fetch from 'node-fetch'; | |||
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify whether node-fetch is declared for the runtimes that import src/lib/stellar/index.ts.
set -euo pipefail
rg -n "from ['\"]node-fetch['\"]|require\\(['\"]node-fetch['\"]\\)" .
fd -t f '^(package\.json|deno\.jsonc?|import_map\.json)$' . --exec sh -c '
echo "--- $1"
sed -n "1,220p" "$1"
' sh {}Repository: ACTA-Team/news-acta
Length of output: 1472
Avoid node-fetch; use native fetch.
node-fetch is imported in src/lib/stellar/index.ts but is missing from package.json. As this project uses Next.js, the global fetch API is natively available, rendering the external dependency unnecessary.
Remove the unused import to prevent potential build failures:
Proposed diff
-import fetch from 'node-fetch';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import fetch from 'node-fetch'; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/stellar/index.ts` at line 1, Remove the unnecessary node-fetch import
from the stellar module and use the native global fetch instead. Update
src/lib/stellar/index.ts by deleting the import statement and make sure any
references in the module rely on the built-in fetch API so the code stays
compatible with Next.js without requiring an extra dependency.
| type: string; | ||
| transaction_hash?: string; | ||
| source_account?: string; | ||
| [key: string]: any; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
cat -n src/lib/stellar/index.tsRepository: ACTA-Team/news-acta
Length of output: 2590
🏁 Script executed:
python3 - <<'PY'
import json
# Simulate the TypeScript index signature behavior
# The original: [key: string]: any
# TS Behavior: A property accessed with [string] can accept value of type 'any'.
# Replacing with 'unknown':
# 1. The interface becomes: { [key: string]: unknown }.
# 2. Assigning an object `data` (JSON) to a variable of this type is valid (record).
# 3. The issue is `map((r: any) => r as HorizonOperation)`.
# If the interface has `[key: string]: any`, and data comes in, mapping `r` to `HorizonOperation` is fine.
# If we change index to `unknown`, the variable `r` inside map (inferred from `records`) would be `unknown`.
# `r as HorizonOperation` on `unknown` is valid.
# 4. However, the proposed fix in the original comment suggests a structural type `HorizonOperationsResponse`.
# 5. Let's verify if the specific `no-explicit-any` check flags the index signature.
# Yes, in strict mode `any` in an index signature triggers `no-explicit-any` if it's not a workaround.
# But wait, does the map usage need fixing?
# Line 25: const ops: HorizonOperation[] = (data._embedded?.records || []).map((r: any) => r as HorizonOperation);
# If we fix line 9 to 'unknown', line 25's `(r: any)` is still an explicit `any`.
# The original comment says "lines 9-9 and 25-25".
# If index signature is `any`, it flags line 9.
# If parameter `r` is `any`, it flags line 25.
# Proposed change:
# Line 9: [key: string]: unknown; -> This is a known workaround for unknown additional props but often requires type narrowing when accessing.
# BUT, the `no-explicit-any` rule *specifically* prohibits `any`. `unknown` is the replacement.
# However, changing line 9 to `unknown` makes line 25's `r` type `unknown` (implicitly) or `any` (explicitly).
# The proposed fix in the original comment adds an interface structure for better typing.
# Let's check if the proposed fix is better practice.
# Current: `[key: string]: any` allows everything.
# Better: `Record<string, unknown>` or specific types.
# The code uses `r as HorizonOperation`. If `data._embedded.records` has `unknown` entries, we still need to cast or verify.
# The `any` on index signature is definitely a lint violation if `no-explicit-any` is on.
# The `any` on the map parameter `r` is ALSO a lint violation.
# Conclusion: The issue is real on both lines.
# Line 9: `[key: string]: any` -> Replace with `Record<string, unknown>` or `[key: string]: unknown`.
# Line 25: `r: any` -> Remove the type annotation and let TS infer `unknown` from the array, or use the cast directly.
# The original comment suggests `[key: string]: unknown` and updating the response type.
# I will rewrite the comment to include both fixes (line 9 and 25) clearly.
print("Verification complete: Both line 9 and 25 contain explicit 'any' triggering lint rules.")
PYRepository: ACTA-Team/news-acta
Length of output: 149
Replace explicit any with unknown and remove unnecessary type annotation.
Update the HorizonOperation index signature and remove the explicit any type from the map callback to satisfy @typescript-eslint/no-explicit-any.
src/lib/stellar/index.ts
export interface HorizonOperation {
// ...
- [key: string]: any;
+ [key: string]: unknown;
}- const ops: HorizonOperation[] = (data._embedded?.records || []).map((r: any) => r as HorizonOperation);
+ const ops: HorizonOperation[] = (data._embedded?.records || []).map((r) => r as HorizonOperation);The parameter r will be inferred as unknown from the array source. The subsequent as HorizonOperation cast remains valid to narrow the type before use.
🧰 Tools
🪛 ESLint
[error] 9-9: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🪛 GitHub Check: lint-and-build
[failure] 9-9:
Unexpected any. Specify a different type
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/stellar/index.ts` at line 9, The `HorizonOperation` shape currently
uses an explicit `any` index signature and the map callback also relies on an
unnecessary explicit `any`, which should be removed to satisfy the lint rule.
Update the `HorizonOperation` type in `index.ts` to use `unknown` for the index
signature, and remove the explicit type annotation from the `r` parameter in the
mapping logic so it can be inferred from the array source; keep the existing `as
HorizonOperation` cast where the value is narrowed for use.
Source: Linters/SAST tools
| const { data: accounts } = await supabase.from('monitored_accounts').select('*').eq('active', true); | ||
| if (!accounts) return { processed: 0 }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
cat -n supabase/functions/activity_scanner/index.tsRepository: ACTA-Team/news-acta
Length of output: 2461
🏁 Script executed:
cd /workspace/supabase/functions/activity_scanner
cat -n index.tsRepository: ACTA-Team/news-acta
Length of output: 290
🏁 Script executed:
cat -n "supabase/functions/activity_scanner/index.ts"Repository: ACTA-Team/news-acta
Length of output: 2461
🏁 Script executed:
cat -n "src/lib/supabase/admin.ts"Repository: ACTA-Team/news-acta
Length of output: 644
🏁 Script executed:
python3 - <<'PY'
import json
# Simulating Supabase client response for select().eq()
# If query fails (e.g., network issue, permission denied, DB error),
# the standard supabase-js response is: error != null, and data is null.
# If query succeeds but finds no rows, response is: error == null, data == [].
# Current Code Logic at lines 10-11:
# const { data: accounts } = await supabase.from('monitored_accounts').select('*').eq('active', true);
# if (!accounts) return { processed: 0 };
# Scenario 1: Query FAILED (e.g., 500 error)
# Response: { data: null, error: { message: '...', ... } }
# Variable accounts = null
# if (!accounts) is TRUE -> Returns { processed: 0 } silently.
# CONSEQUENCE: The caller (e.g., Scheduled Function) sees success but 0 processed.
# An outage is masked as "nothing to do".
# Scenario 2: Query SUCCESS, no active accounts
# Response: { data: [], error: null }
# Variable accounts = []
# typeof [] is "object", ![] is FALSE.
# Loop `for (const acct of accounts)` iterates 0 times.
# Returns { processed: 0 } explicitly.
# CONSEQUENCE: Correctly reflects "no work done".
# The bug is specific to Scenario 1 (Error vs No Results).
# The proposed fix in the original comment checks for `accountsError` explicitly.
# Let's verify if the file has any import for `SupabaseResponse` type or similar
# to see if error destructuring is expected elsewhere.
# We also need to confirm if the current code throws on error or swallows it.
print("Analysis Result:")
print("The current code destructures ONLY `data`.")
print("When Supabase query fails, `data` is `null`.")
print("The check `if (!accounts)` treats `null` (error) and potentially other falsy values identically to 'no data'.")
print("However, if the query returns an empty array (success), `data` is `[]`.")
print("'!' converts `[]` to `false`, so `if (!accounts)` is FALSE for empty results.")
print("Wait, let's re-verify JS truthy/falsy.")
print(f"Boolean({[]}): {bool([])}") # This is True (Objects are truthy)
# Actually `![]` is false. So `if (!accounts)` with accounts=[] is FALSE.
# The code proceeds to `for (const acct of [])` which does nothing.
# It returns `{ processed: 0 }` at the end. That is fine.
# But for Error: accounts = null. `!null` is True.
# Code returns `{ processed: 0 }` immediately.
# Error is ignored. No alerting, no propagation.
# Conclusion: The original review comment is correct.
# The missing error check swallows database failures.
PYRepository: ACTA-Team/news-acta
Length of output: 580
🏁 Script executed:
node - <<'JS'
// Verify the truthiness of an empty array in JavaScript
const emptyArr = [];
console.log("Boolean([]):", Boolean(emptyArr));
console.log("![]:", !emptyArr);
console.log("Type of []:", typeof emptyArr);
// Verify the logic of the current code
// Scenario: Query fails
const errorData = null;
console.log("\nScenario: Query Failed (data is null)");
// Simulating: const { data: accounts } = { data: null, error: ... };
const accountsErrorCase = errorData;
if (!accountsErrorCase) {
console.log("-> Current: Treates as 'no work', returns { processed: 0 }");
} else {
console.log("-> Current: Proceeds to loop");
}
// Scenario: Query succeeds, no rows
const successData = [];
console.log("\nScenario: Success, empty results (data is [])");
console.log("Current check '!successData':", !successData);
if (!successData) {
console.log("-> Current: Treats as 'no work'");
} else {
console.log("-> Current: Proceeds to loop");
for (const item of successData) {
console.log("Processing item");
}
console.log("-> Loop finished empty, returns { processed: 0 }");
}
JSRepository: ACTA-Team/news-acta
Length of output: 464
🏁 Script executed:
sed -n '18p;27p;54p' "supabase/functions/activity_scanner/index.ts"Repository: ACTA-Team/news-acta
Length of output: 269
🏁 Script executed:
grep -n "error" "supabase/functions/activity_scanner/index.ts"Repository: ACTA-Team/news-acta
Length of output: 222
Hide ingestion outages when monitored_accounts query fails.
Destructure the error property from the query response and throw it if present. Currently, if the database query fails, data is null, causing the check if (!accounts) to trigger, which silently returns { processed: 0 } instead of surfacing the failure.
Proposed fix
- const { data: accounts } = await supabase.from('monitored_accounts').select('*').eq('active', true);
- if (!accounts) return { processed: 0 };
+ const { data: accounts, error: accountsError } = await supabase
+ .from('monitored_accounts')
+ .select('*')
+ .eq('active', true);
+
+ if (accountsError) throw accountsError;
+ if (!accounts?.length) return { processed: 0 };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { data: accounts } = await supabase.from('monitored_accounts').select('*').eq('active', true); | |
| if (!accounts) return { processed: 0 }; | |
| const { data: accounts, error: accountsError } = await supabase | |
| .from('monitored_accounts') | |
| .select('*') | |
| .eq('active', true); | |
| if (accountsError) throw accountsError; | |
| if (!accounts?.length) return { processed: 0 }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@supabase/functions/activity_scanner/index.ts` around lines 10 - 11, The
monitored_accounts fetch in activity_scanner currently treats any null data as
an empty result, which hides real query failures. Update the supabase query
handling in the activity scanning flow to destructure the error alongside data,
and in the same block throw or propagate that error before checking accounts.
Keep the existing processed fallback only for the true no-data case, and make
the change in the logic around the monitored_accounts query response.
| const classification = classifyOperation(op as any); | ||
| // basic dedupe: by tx_hash | ||
| const txHash = (op as any).transaction_hash || null; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
cat -n supabase/functions/activity_scanner/index.tsRepository: ACTA-Team/news-acta
Length of output: 2461
🏁 Script executed:
grep -r "type HorizonOperation" --include="*.ts"Repository: ACTA-Team/news-acta
Length of output: 157
🏁 Script executed:
grep -r "function classifyOperation" --include="*.ts" -A 5 --include="*.d.ts"Repository: ACTA-Team/news-acta
Length of output: 758
🏁 Script executed:
cat -n src/lib/stellar/index.tsRepository: ACTA-Team/news-acta
Length of output: 2590
Remove the redundant any casts and unnecessary operator usage.
fetchOperationsSince returns HorizonOperation[], so op is already properly typed. HorizonOperation includes transaction_hash and allows index access any, making the as any casts redundant.
const classification = classifyOperation(op);
// basic dedupe: by tx_hash
- const txHash = (op as any).transaction_hash || null;
+ const txHash = op.transaction_hash ?? null;Also, note that classifyOperation accesses op.account (line 32 of stellar/index.ts), which is not explicitly defined in the HorizonOperation interface, yet the code relies on the [key: string]: any index signature. Ensure that dynamic property access is handled consistently.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const classification = classifyOperation(op as any); | |
| // basic dedupe: by tx_hash | |
| const txHash = (op as any).transaction_hash || null; | |
| const classification = classifyOperation(op); | |
| // basic dedupe: by tx_hash | |
| const txHash = op.transaction_hash ?? null; |
🧰 Tools
🪛 ESLint
[error] 19-19: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 21-21: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🪛 GitHub Check: lint-and-build
[failure] 21-21:
Unexpected any. Specify a different type
[failure] 19-19:
Unexpected any. Specify a different type
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@supabase/functions/activity_scanner/index.ts` around lines 19 - 21, Remove
the redundant `as any` casts in `activity_scanner/index.ts` by using the
existing `HorizonOperation` type from `fetchOperationsSince` directly; update
the `classifyOperation` call and `transaction_hash` access to rely on the typed
operation object. Also review `classifyOperation` in `stellar/index.ts` so
dynamic fields like `account` are accessed consistently through the interface’s
index signature rather than unnecessary casts, keeping the typing aligned across
both functions.
Source: Linters/SAST tools
| // basic dedupe: by tx_hash | ||
| const txHash = (op as any).transaction_hash || null; | ||
| const { data: existing } = await supabase | ||
| .from('activity_events') | ||
| .select('id') | ||
| .eq('tx_hash', txHash) | ||
| .limit(1); | ||
| if (existing && existing.length) continue; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make dedupe operation-scoped and atomic.
Dedupe by tx_hash drops additional operations from the same transaction, and the select-then-insert pattern can race across concurrent scanner runs. Store a Horizon operation identifier/paging token and enforce a unique constraint/upsert around that key.
🧰 Tools
🪛 ESLint
[error] 21-21: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🪛 GitHub Check: lint-and-build
[failure] 21-21:
Unexpected any. Specify a different type
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@supabase/functions/activity_scanner/index.ts` around lines 20 - 27, The
dedupe in activity_scanner/index.ts is too broad and non-atomic: it currently
uses transaction_hash in the existing lookup, which can suppress distinct
operations from the same transaction and can race between concurrent runs.
Update the scan/insert flow to use a Horizon operation-scoped identifier or
paging token in the activity_events record, and enforce uniqueness on that key
instead of tx_hash. Then switch the create path to an atomic
upsert/insert-with-conflict-handling around the same identifier so duplicate
scanner runs cannot insert the same operation twice.
| await supabase.from('activity_events').insert([ | ||
| { | ||
| account_id: acct.id, | ||
| source_account: op.source_account || acct.stellar_address, | ||
| event_type: classification.eventType, | ||
| significance: classification.significance, | ||
| raw_data: op, | ||
| summary: classification.summary, | ||
| tx_hash: txHash, | ||
| detected_at: new Date().toISOString(), | ||
| }, | ||
| ]); | ||
| processed++; | ||
| } | ||
|
|
||
| // update cursor if we got one | ||
| if (nextCursor) { | ||
| await supabase.from('monitored_accounts').update({ last_cursor: nextCursor }).eq('id', acct.id); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Check write errors before incrementing or advancing the cursor.
Failed inserts/updates are currently ignored, so processed can be inflated and last_cursor can advance past events that were never persisted.
Proposed fix
- await supabase.from('activity_events').insert([
+ const { error: insertError } = await supabase.from('activity_events').insert([
{
account_id: acct.id,
source_account: op.source_account || acct.stellar_address,
@@
detected_at: new Date().toISOString(),
},
]);
+ if (insertError) throw insertError;
processed++;
@@
- await supabase.from('monitored_accounts').update({ last_cursor: nextCursor }).eq('id', acct.id);
+ const { error: cursorError } = await supabase
+ .from('monitored_accounts')
+ .update({ last_cursor: nextCursor })
+ .eq('id', acct.id);
+ if (cursorError) throw cursorError;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await supabase.from('activity_events').insert([ | |
| { | |
| account_id: acct.id, | |
| source_account: op.source_account || acct.stellar_address, | |
| event_type: classification.eventType, | |
| significance: classification.significance, | |
| raw_data: op, | |
| summary: classification.summary, | |
| tx_hash: txHash, | |
| detected_at: new Date().toISOString(), | |
| }, | |
| ]); | |
| processed++; | |
| } | |
| // update cursor if we got one | |
| if (nextCursor) { | |
| await supabase.from('monitored_accounts').update({ last_cursor: nextCursor }).eq('id', acct.id); | |
| const { error: insertError } = await supabase.from('activity_events').insert([ | |
| { | |
| account_id: acct.id, | |
| source_account: op.source_account || acct.stellar_address, | |
| event_type: classification.eventType, | |
| significance: classification.significance, | |
| raw_data: op, | |
| summary: classification.summary, | |
| tx_hash: txHash, | |
| detected_at: new Date().toISOString(), | |
| }, | |
| ]); | |
| if (insertError) throw insertError; | |
| processed++; | |
| } | |
| // update cursor if we got one | |
| if (nextCursor) { | |
| const { error: cursorError } = await supabase | |
| .from('monitored_accounts') | |
| .update({ last_cursor: nextCursor }) | |
| .eq('id', acct.id); | |
| if (cursorError) throw cursorError; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@supabase/functions/activity_scanner/index.ts` around lines 29 - 46, The
activity scanner currently ignores failures from the activity_events insert and
monitored_accounts cursor update, so processed can be incremented and
last_cursor advanced even when persistence fails. In the activity_scanner flow
around the supabase.from('activity_events') insert and the subsequent cursor
update, check the returned error/result before incrementing processed or setting
last_cursor, and only advance state after a successful write; otherwise
log/handle the failure and leave the cursor unchanged so the same events can be
retried.
| create table if not exists public.activity_events ( | ||
| id uuid primary key default gen_random_uuid(), | ||
| account_id uuid not null references public.monitored_accounts(id) on delete cascade, | ||
| source_account text not null, | ||
| event_type text not null, | ||
| significance text not null, | ||
| raw_data jsonb not null default '{}'::jsonb, | ||
| summary text, | ||
| processed boolean not null default false, | ||
| draft_article_id uuid, | ||
| detected_at timestamptz not null default now(), | ||
| tx_hash text | ||
| ); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Constrain event_type and significance in the database.
The TS layer already limits these fields to a fixed set, but the table still accepts arbitrary text. That lets malformed rows slip in and breaks the contract this schema is trying to establish.
Possible fix
create table if not exists public.activity_events (
id uuid primary key default gen_random_uuid(),
account_id uuid not null references public.monitored_accounts(id) on delete cascade,
source_account text not null,
- event_type text not null,
- significance text not null,
+ event_type text not null check (event_type in ('contract_deploy', 'payment', 'trust_change', 'contract_invoke', 'data_entry', 'account_create')),
+ significance text not null check (significance in ('low', 'medium', 'high', 'critical')),
raw_data jsonb not null default '{}'::jsonb,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| create table if not exists public.activity_events ( | |
| id uuid primary key default gen_random_uuid(), | |
| account_id uuid not null references public.monitored_accounts(id) on delete cascade, | |
| source_account text not null, | |
| event_type text not null, | |
| significance text not null, | |
| raw_data jsonb not null default '{}'::jsonb, | |
| summary text, | |
| processed boolean not null default false, | |
| draft_article_id uuid, | |
| detected_at timestamptz not null default now(), | |
| tx_hash text | |
| ); | |
| create table if not exists public.activity_events ( | |
| id uuid primary key default gen_random_uuid(), | |
| account_id uuid not null references public.monitored_accounts(id) on delete cascade, | |
| source_account text not null, | |
| event_type text not null check (event_type in ('contract_deploy', 'payment', 'trust_change', 'contract_invoke', 'data_entry', 'account_create')), | |
| significance text not null check (significance in ('low', 'medium', 'high', 'critical')), | |
| raw_data jsonb not null default '{}'::jsonb, | |
| summary text, | |
| processed boolean not null default false, | |
| draft_article_id uuid, | |
| detected_at timestamptz not null default now(), | |
| tx_hash text | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@supabase/migrations/0004_activity_events.sql` around lines 5 - 17, The
activity_events table currently leaves event_type and significance as
unrestricted text, so add database-level constraints in the migration to match
the TypeScript allowed values. Update the public.activity_events definition to
constrain both columns, using CHECK constraints or enum-backed types, and keep
the change localized around the activity_events table creation so the schema
contract is enforced at the database layer.
🚀 ACTA Pull Request
Mark with an
xall the checkboxes that apply (like[x])📌 Type of Change
📝 Changes description
📸 Evidence (A Loom/Cap video is required as evidence, we WON'T merge if there's no proof)
⏰ Time spent breakdown
🌌 Comments
Thank you for contributing to ACTA! We hope you can continue contributing to this project.
Summary by CodeRabbit
New Features
Bug Fixes
Chores